Removing almost unused ConsoleExtensions - #2103

Possibly this should assume colors are supported if console redirection cannot be checked. See #1851.

In 908accd, ConsoleExtensions was changed so that, if Platform.IsWasm, then it sets Console.ForegroundColor without checking Console.IsOutputRedirected. In dotnet/runtime#35555 however, IsOutputRedirectedCore() returns false without throwing, whereas ForegroundColor throws PlatformNotSupportedException. So the original wasm logic in ConsoleExtensions doesn't seem necessary any more, although I did not test.

The only PlatformNotSupportedException-throwing IsOutputRedirectedCore method seems to be in https://github.com/dotnet/runtime/blob/30b879924a47d8660d5f4b14ff581ce40ae076f2/src/libraries/System.Console/src/System/ConsolePal.Wasi.cs, in which ForegroundColor likewise throws PlatformNotSupportedException. So okay, it's correct to assume colors are not supported if IsOutputRedirected throws.

@KalleOlaviNiemitalo thank you for your feedback, I've changed the code so it checks the OS first, then the IsOutputRedirected which won't throw after the OS check

Which of those OS checks returns true on WASI?

The "browser" checks are WASM checks:

OperatingSystem.IsBrowser(), RuntimeInformation.IsOSPlatform(OSPlatform.Create("BROWSER")

If an application is designed to parse and invoke commands from custom sources like network sockets, and it should not modify System.Console.ForegroundColor during these operations (because such changes might mess up output from a different thread), then it apparently needs to avoid UseParseErrorReporting entirely and avoid UseExceptionHandler with the default handler. That's tolerable but not quite obvious.

If you're eventually going to have TextWriter Error, then perhaps you can compare Error == System.Console.Error and change Console.ForegroundColor only in that matches. Or have some bool AllowConsoleColor property in CommandLineConfiguration.

Thinking on — if class CliConfiguration is going to have TextWriter Error, then it can have bool UseErrorColor { get; set; } such that…

• If UseErrorColor has not been set, then its getter computes the value from this.Error == System.Console.Error && !System.Console.IsErrorRedirected, defaulting to false on PlatformNotSupportedException. Does not cache the result into a field of CliConfiguration because it's going to be used only once per error. Instead, each method that reads this property should save the result to a local so that it restores the colours if and only if it has set them.
• If UseErrorColor has been set, then trust the application-assigned value when deciding whether to set the foreground color, and do not check whether error has been redirected.

Not sure about IsErrorRedirected vs. IsOutputRedirected. The error text should go to the CliConfiguration.Error TextWriter, but if System.Console.ForegroundColor outputs ECMA-48 control sequences instead of using the Windows console API, I guess it cannot be told whether those should go to Console.Error or Console.Out. So perhaps the UseErrorColor getter should check this.Error == System.Console.Error && !System.Console.IsErrorRedirected && !System.Console.IsOutputRedirected. I'm omitting this.Out == System.Console.Out because errors should never go to this.Out.

When https://github.com/dotnet/runtime/blob/5edef4b20babd4c3ddac7460e536f86fd0f2d724/src/libraries/System.Console/src/System/ConsolePal.Unix.cs sets colors, it does lock (Console.Out) but writes to STDOUT_FILENO. So Console.SetOut(TextWriter) affects the locking but does not affect where the SGR control sequence goes.